Skip to content

Wait for MPS daemon readiness before advertising shared resources - #1946

Open
jonathan-meiri wants to merge 1 commit into
NVIDIA:mainfrom
jonathan-meiri:mps-wait-for-ready
Open

Wait for MPS daemon readiness before advertising shared resources#1946
jonathan-meiri wants to merge 1 commit into
NVIDIA:mainfrom
jonathan-meiri:mps-wait-for-ready

Conversation

@jonathan-meiri

Copy link
Copy Markdown

Summary

The device plugin can advertise MPS-shared resources to kubelet before the MPS daemon has finished configuring per-device memory limits and thread percentages. A pod scheduled in that window runs against MPS with the daemon defaults (no pinned memory limit, 100% threads) instead of the configured limits — silently bypassing the intended isolation.

This gates readiness on the MPS daemon's .ready file, closing the two existing TODOs in waitForDaemon.

Contributed by @Meiri28 on behalf of @runatom-ai.

The race

internal/plugin/server.go:Start() calls plugin.mps.waitForDaemon() before Serve() + Register(). Previously waitForDaemon ran a single AssertHealthy():

// TODO: Check the .ready file here.
// TODO: Have some retry strategy here.
if err := m.daemon.AssertHealthy(); err != nil { ... }

AssertHealthy() issues get_default_active_thread_percentage, which only proves the control pipe is responsive. In Daemon.Start() the ordering is:

  1. mpsControlBin -d starts → pipe becomes responsive (AssertHealthy passes here)
  2. per-device pinned memory limits applied
  3. active thread percentage applied
  4. .ready file created (after all daemons' Start() return)

If the single check lands between (1) and (4), the plugin registers as ready while the configured MPS limits are not yet in place.

The MPS control daemon already creates a node-global .ready file only after full initialization, but nothing consumed it — the two TODOs noted exactly this gap. Both components share the same /mps hostPath mount, so the file is visible to the device plugin.

Changes

  • mps/root.go: add Root.ReadyFilePath() so the marker path has a single definition.
  • mps-control-daemon/main.go: create/remove .ready via ReadyFilePath() instead of the hardcoded /mps/.ready, keeping writer and reader in sync.
  • mps/daemon.go: add Daemon.Ready(), reporting whether the .ready file exists.
  • internal/plugin/mps.go: rewrite waitForDaemon to poll checkDaemonReady (Ready() and AssertHealthy()) every 5s up to a 5m bound. On timeout the caller fails and is retried by the plugin manager, so the bound is per-attempt.

Test plan

  • go test ./... — full suite passes; new TestReadyFilePath and TestDaemonReady cover the readiness primitive (readiness reflects .ready file existence).
  • make build, make check-modules, gofmt — clean.

Commits are DCO-signed.

@copy-pr-bot

copy-pr-bot Bot commented Aug 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jonathan-meiri
jonathan-meiri force-pushed the mps-wait-for-ready branch 2 times, most recently from 6966a94 to bfcb598 Compare August 6, 2026 13:13
@myeolenv

myeolenv commented Aug 6, 2026

Copy link
Copy Markdown

Thanks for the contribution. We will review this soon.

The device plugin's waitForDaemon only ran a single AssertHealthy check
before serving and registering the resource with kubelet. AssertHealthy
issues get_default_active_thread_percentage, which only proves the MPS
control pipe is responsive — and the pipe becomes responsive at
Daemon.Start (mpsControlBin -d) before the per-device pinned memory
limits and active thread percentage are applied. A pod scheduled in
that window starts against MPS with the daemon defaults (no pinned
memory limit, 100% threads) rather than the configured limits,
silently bypassing the intended isolation.

The MPS control daemon already creates a node-global .ready file, but
only after every daemon's full initialization completes. Nothing
consumed it (the two TODOs in waitForDaemon noted exactly this), so the
readiness signal was unused.

Gate readiness on that file:

  - Add Root.ReadyFilePath so the marker path has a single definition,
    and use it in the MPS control daemon for both create and remove
    instead of the hardcoded "/mps/.ready".
  - Add Daemon.Ready, which reports whether the .ready file exists.
  - Rewrite waitForDaemon to poll checkDaemonReady (Ready AND
    AssertHealthy) every 5s up to a 5m bound, replacing the single
    unconditional AssertHealthy. On timeout the caller fails and is
    retried by the plugin manager, so the bound is per-attempt.

This closes both TODOs and ensures shared MPS resources are not
advertised until the daemon is fully configured.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants